packages/core/src/2d/text/TextUtils.ts (2)

274-274: LGTM with documentation suggestion

The method signature change is correct. Consider adding JSDoc to document the new parameter type.

+/**
+ * Measures text without wrapping for both TextRenderer and UIText.
+ * @param renderer - The text renderer (TextRenderer or UIText)
+ * @returns TextMetrics containing measurement information
+ */
 static measureTextWithoutWrap(renderer: TextRenderer | UIText): TextMetrics {

---

Line range hint 100-274: Consider adding integration tests for UIText measurements

Since these methods now support both TextRenderer and UIText, it would be valuable to add integration tests that verify:

1. Text measurement consistency between TextRenderer and UIText
2. Edge cases with different text styles and overflow modes
3. Performance implications of supporting both types

This will help ensure compatibility and prevent regressions.

packages/core/src/ui/UIText.ts (2)

323-323: Address the TODO: Optimize world positions buffer update

There's a TODO comment at line 323 indicating that optimization is needed: //@todo: Always update world positions to buffer, should opt. Would you like assistance in implementing this optimization or opening a GitHub issue to track it?

---

514-514: Consider splitting assignments for better readability

At line 514, the line combines multiple assignments in one statement:

const charRenderInfo = (charRenderInfos[renderElementCount++] = charRenderInfoPool.get());

This reduces code clarity. Consider splitting the assignments.

Suggested change:

- const charRenderInfo = (charRenderInfos[renderElementCount++] = charRenderInfoPool.get());
+ const charRenderInfo = charRenderInfoPool.get();
+ charRenderInfos[renderElementCount++] = charRenderInfo;

🧰 Tools

🪛 Biome

[error] 514-514: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Verify UIText implements required properties and methods

# Search for UIText class definition and implementation
ast-grep --pattern 'class UIText {
  $$$
  _getSubFont() {
    $$$
  }
  $$$
}'

# Search for width, height, lineSpacing, and overflowMode properties
rg -A 5 'class UIText' 

#!/bin/bash
# Search for required properties and methods in UIText class
rg -A 2 "(_getSubFont|width\s*:|height\s*:|lineSpacing\s*:|overflowMode\s*:)" packages/core/src/ui/UIText.ts

# Search for property declarations
ast-grep --pattern 'class UIText extends UIRenderer {
  $$$
  private _width: $_
  $$$
}'

# Check if UIText extends TextRenderer or has similar interface
rg -A 2 "interface.*Text" packages/core/src/2d/text/

#!/bin/bash
# Check TextRenderer class and its properties
rg -A 5 "class TextRenderer" packages/core/src/2d/text/

# Check if UIText implements or extends any text-related interfaces
rg "class UIText.*implements" packages/core/src/ui/UIText.ts

# Look for any shared interfaces between TextRenderer and UIText
ast-grep --pattern 'interface $_ {
  $$$
  _getSubFont(): $_
  $$$
}'

packages/core/src/ui/interactive/transition/TransitionType.ts (1)

1-6: Consider adding JSDoc comments to document each transition type.

Adding documentation would improve maintainability by explaining the purpose and behavior of each transition type.

+/**
+ * Defines the types of transitions available for UI elements.
+ */
 export enum TransitionType {
+  /** No transition effect */
   None,
+  /** Color-based transition effect */
   Color,
+  /** Sprite-based transition effect */
   Sprite,
+  /** Scale-based transition effect */
   Scale
 }

packages/core/src/ui/interactive/InteractiveState.ts (2)

1-7: Consider renaming 'Disable' to 'Disabled' for grammatical consistency.

The enum implementation looks good and covers all essential UI interaction states. However, 'Disable' should be 'Disabled' to maintain consistent grammar (adjective form) with other state names.

 export enum InteractiveState {
   None,
   Normal,
   Pressed,
   Hover,
-  Disable
+  Disabled
 }

---

1-1: Add JSDoc documentation for the enum.

Since this enum is a core part of the GUI infrastructure used across multiple components, please add JSDoc documentation explaining:

• The purpose of each state
• Usage examples
• State transition expectations

+/**
+ * Represents the interactive states of UI components.
+ * 
+ * @remarks
+ * - None: Initial/unset state
+ * - Normal: Default interactive state
+ * - Pressed: Component is being actively pressed/clicked
+ * - Hover: Mouse is hovering over the component
+ * - Disabled: Component is non-interactive
+ */
 export enum InteractiveState {

packages/core/src/ui/interface/IGroupElement.ts (2)

4-8: Add JSDoc documentation and consider using readonly modifiers.

The interface definition could benefit from documentation explaining its purpose and usage. Additionally, consider marking properties that shouldn't change after initialization as readonly.

Here's the suggested improvement:

+/**
+ * Interface for elements that can be part of a UIGroup.
+ * Extends ICanvasElement to provide group-specific functionality.
+ */
 export interface IGroupElement extends ICanvasElement {
-  _group: UIGroup;
-  _indexInGroup: number;
+  readonly _group: UIGroup;
+  readonly _indexInGroup: number;
   _onGroupModify(flag: GroupModifyFlags): void;
 }

---

4-8: Consider encapsulating internal implementation details.

The interface exposes implementation details through underscore-prefixed properties and methods. This could make it harder to maintain and refactor the codebase in the future.

Consider:

1. Moving internal details to a separate internal interface
2. Using a more public-facing API design
3. Using TypeScript's private fields (#) if these are truly meant to be private

Example approach:

export interface IGroupElement extends ICanvasElement {
  readonly group: UIGroup;
  readonly indexInGroup: number;
  onModify(flag: GroupModifyFlags): void;
}

// Internal interface for implementation
interface IGroupElementInternal extends IGroupElement {
  _group: UIGroup;
  _indexInGroup: number;
  _onGroupModify(flag: GroupModifyFlags): void;
}

packages/core/src/ui/interface/IUIGraphics.ts (1)

5-10: Consider interface segregation for better modularity.

The current interface combines multiple responsibilities (depth management, hit testing, and group behavior). Consider splitting this into more focused interfaces:

interface IDepthManagement {
  depth: number;
}

interface IRaycastable {
  raycastEnable: boolean;
  raycastPadding: Vector4;
  raycast(ray: Ray, out: HitResult, distance: number): boolean;
}

export interface IUIGraphics extends IGroupElement, IDepthManagement, IRaycastable {}

This would:

• Make it easier to implement each behavior independently
• Allow for better composition of UI elements
• Improve testability of each concern

Would you like me to create a GitHub issue to track this refactoring suggestion?

packages/core/src/ui/index.ts (2)

1-15: LGTM! Well-organized exports structure.

The exports are logically grouped and follow a clear pattern, providing a good foundation for the GUI infrastructure. The separation between core enums, UI components, and transitions is clear and maintainable.

Consider adding JSDoc comments to document the main export groups:

+/** Core UI enums */
 export { UIGroup } from "./UIGroup";
 export { CanvasRenderMode } from "./enums/CanvasRenderMode";
 export { ResolutionAdaptationStrategy } from "./enums/ResolutionAdaptationStrategy";

+/** UI Components */
 export { Button } from "./Button";
 export { UICanvas } from "./UICanvas";
 export { UIImage } from "./UIImage";
 export { UIRenderer } from "./UIRenderer";
 export { UIText } from "./UIText";
 export { UITransform } from "./UITransform";

+/** Transition Components */
 export { ColorTransition } from "./interactive/transition/ColorTransition";
 export { ScaleTransition } from "./interactive/transition/ScaleTransition";
 export { SpriteTransition } from "./interactive/transition/SpriteTransition";
 export { Transition } from "./interactive/transition/Transition";

---

5-15: Consider sorting exports within groups.

While the current organization is clear, consider sorting the exports alphabetically within their groups to make it easier to locate specific exports as the list grows.

 export { Button } from "./Button";
-export { UICanvas } from "./UICanvas";
-export { UIImage } from "./UIImage";
-export { UIRenderer } from "./UIRenderer";
-export { UIText } from "./UIText";
-export { UITransform } from "./UITransform";
+export { UICanvas } from "./UICanvas";
+export { UIImage } from "./UIImage";
+export { UIRenderer } from "./UIRenderer";
+export { UIText } from "./UIText";
+export { UITransform } from "./UITransform";

-export { ColorTransition } from "./interactive/transition/ColorTransition";
-export { ScaleTransition } from "./interactive/transition/ScaleTransition";
-export { SpriteTransition } from "./interactive/transition/SpriteTransition";
-export { Transition } from "./interactive/transition/Transition";
+export { ColorTransition } from "./interactive/transition/ColorTransition";
+export { ScaleTransition } from "./interactive/transition/ScaleTransition";
+export { SpriteTransition } from "./interactive/transition/SpriteTransition";
+export { Transition } from "./interactive/transition/Transition";

packages/core/src/ui/interactive/transition/ScaleTransition.ts (2)

5-12: Consider enhancing configurability and visual feedback.

The current implementation could benefit from:

1. Constructor parameters to allow customization of scale values and duration
2. Different scale values for hover and disabled states to provide better visual feedback

Consider this implementation:

-  constructor() {
+  constructor(config?: {
+    normal?: number;
+    hover?: number;
+    pressed?: number;
+    disabled?: number;
+    duration?: number;
+  }) {
     super();
-    this._normal = 1;
-    this._hover = 1;
-    this._pressed = 1.2;
-    this._disabled = 1;
-    this._duration = 0.1;
+    this._normal = config?.normal ?? 1;
+    this._hover = config?.hover ?? 1.1;
+    this._pressed = config?.pressed ?? 1.2;
+    this._disabled = config?.disabled ?? 0.9;
+    this._duration = config?.duration ?? 0.2;
   }

---

18-20: Consider using easing functions for smoother transitions.

The current linear interpolation might not provide the most visually pleasing transition. Consider implementing easing functions for smoother animations.

Example implementation:

protected override _updateCurrentValue(srcValue: number, destValue: number, weight: number): void {
  // Ease in-out quad function
  const easeWeight = weight < 0.5
    ? 2 * weight * weight
    : 1 - Math.pow(-2 * weight + 2, 2) / 2;
  this._currentValue = (destValue - srcValue) * easeWeight + srcValue;
}

packages/core/src/ui/interactive/transition/ColorTransition.ts (2)

6-15: Consider documenting color states and extracting constants.

The color states and transition duration would benefit from documentation and constant definitions.

Consider applying these improvements:

+/** Default transition duration in seconds */
+private static readonly DEFAULT_DURATION = 0.1;
+
+/** Default color states for different interaction states */
+private static readonly COLORS = {
+  NORMAL: new Color(1, 1, 1, 1),
+  HOVER: new Color(245 / 255, 245 / 255, 245 / 255, 1),
+  PRESSED: new Color(200 / 255, 200 / 255, 200 / 255, 1),
+  DISABLED: new Color(200 / 255, 200 / 255, 200 / 255, 1)
+} as const;

 constructor() {
   super();
-  this._normal = new Color(1, 1, 1, 1);
-  this._hover = new Color(245 / 255, 245 / 255, 245 / 255, 1);
-  this._pressed = new Color(200 / 255, 200 / 255, 200 / 255, 1);
-  this._disabled = new Color(200 / 255, 200 / 255, 200 / 255, 1);
-  this._duration = 0.1;
+  this._normal = ColorTransition.COLORS.NORMAL.clone();
+  this._hover = ColorTransition.COLORS.HOVER.clone();
+  this._pressed = ColorTransition.COLORS.PRESSED.clone();
+  this._disabled = ColorTransition.COLORS.DISABLED.clone();
+  this._duration = ColorTransition.DEFAULT_DURATION;
   this._currentValue = new Color();
 }

---

17-29: Consider optimizing color object reuse.

The current implementation reuses the color object effectively in _getTargetValueCopy, but _updateCurrentValue could be optimized further.

Consider this optimization:

 protected override _updateCurrentValue(srcValue: Color, destValue: Color, weight: number): void {
   if (weight >= 1) {
     this._currentValue.copyFrom(destValue);
   } else {
-    Color.lerp(srcValue, destValue, weight, this._currentValue);
+    // Avoid potential allocation in Color.lerp by ensuring _currentValue is always used
+    this._currentValue.r = srcValue.r + (destValue.r - srcValue.r) * weight;
+    this._currentValue.g = srcValue.g + (destValue.g - srcValue.g) * weight;
+    this._currentValue.b = srcValue.b + (destValue.b - srcValue.b) * weight;
+    this._currentValue.a = srcValue.a + (destValue.a - srcValue.a) * weight;
   }
 }

packages/core/src/ui/Button.ts (3)

6-6: Consider adding JSDoc for the listeners property.

While the property is well-typed and properly encapsulated, adding JSDoc would improve maintainability by documenting the purpose of this collection.

+  /** Collection of click event listeners managed using SafeLoopArray for safe iteration. */
   private _listeners: SafeLoopArray<IUIListener> = new SafeLoopArray<IUIListener>();

---

8-14: Consider preventing duplicate listener registrations.

The current implementation allows the same listener to be registered multiple times, which could lead to redundant executions.

   addClicked(listener: (event: PointerEventData) => void): void {
+    if (this._listeners.find((value) => value.fn === listener)) {
+      return;
+    }
     this._listeners.push({ fn: listener });
   }

---

38-41: Consider adding JSDoc to document the interface.

While the interface is well-structured, adding documentation would improve maintainability.

+/**
+ * Interface for UI event listeners with destruction tracking.
+ */
 export interface IUIListener {
+  /** The event handling function */
   fn: (event: PointerEventData) => void;
+  /** Flag indicating if the listener has been destroyed */
   destroyed?: boolean;
 }

packages/core/src/input/pointer/emitter/PointerPhysicsEventEmitter.ts (3)

13-49: Consider breaking down the complex raycast processing logic.

The method contains nested loops and multiple conditions that could be extracted into smaller, focused methods for better maintainability.

Consider refactoring into smaller methods:

private _isValidScene(scene: Scene): boolean {
  return scene.isActive && !scene.destroyed;
}

private _isPointerInViewport(x: number, y: number, viewport: { x: number, y: number, width: number, height: number }): boolean {
  return !(x < viewport.x || y < viewport.y || 
          x > viewport.x + viewport.width || 
          y > viewport.y + viewport.height);
}

private _processCamera(camera: Camera, ray: Ray, pointer: Pointer): boolean {
  if (camera.renderTarget) return false;
  
  const { x, y } = pointer.position;
  if (!this._isPointerInViewport(x, y, camera.pixelViewport)) {
    return false;
  }
  
  camera.screenPointToRay(pointer.position, ray);
  return true;
}

---

79-96: Consider caching event data to reduce object creation.

The method creates multiple event data objects with the same parameters. Consider caching the event data for better performance.

private _processUp(pointer: Pointer): void {
  const { _enteredEntity: enteredEntity, _draggedEntity: draggedEntity } = this;
  
  if (enteredEntity) {
    const sameTarget = this._pressedEntity === enteredEntity;
    const eventData = this._createEventData(pointer, enteredEntity, enteredEntity);
    
    this._invokeEntityScripts(enteredEntity, (script: Script) => {
      script.onPointerUp?.(eventData);
      sameTarget && script.onPointerClick?.(eventData);
      script.onPointerDrop?.(eventData);
    });
  }
  // ... rest of the method
}

---

98-115: Apply similar event data caching optimization.

Similar to the _processUp method, consider caching the event data objects to reduce object creation.

packages/core/src/ui/UIUtils.ts (4)

21-37: Add JSDoc documentation for better maintainability.

This method handles complex parent traversal and listener management. Consider adding JSDoc documentation to explain:

• The purpose of the method
• The relationship between entities and listeners
• The significance of the root canvas check

---

47-82: Consider extracting shared logic between registration methods.

The registerElementToCanvas and registerElementToGroup methods share similar patterns for managing indices and elements. Consider extracting the common logic into a private helper method to improve maintainability and reduce code duplication.

Example approach:

private static registerElementToContainer<T extends ICanvasElement | IGroupElement, C extends UICanvas | UIGroup>(
  element: T,
  container: C,
  getPreContainer: (el: T) => C,
  getElements: (c: C) => DisorderedArray<T>,
  updateIndex: (el: T, index: number) => void
): void {
  const preContainer = getPreContainer(element);
  if (preContainer !== container) {
    // ... shared logic here
  }
}

🧰 Tools

🪛 Biome

[error] 53-53: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

---

[error] 72-72: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

---

84-126: Consider using type guard functions for safer type casting.

Replace type casting with type guard functions to improve type safety and readability:

private static isUICanvas(component: Component): component is UICanvas {
  return component._componentType === ComponentType.UICanvas;
}

private static isUIGroup(component: Component): component is UIGroup {
  return component._componentType === ComponentType.UIGroup;
}

Then use them in the methods:

if (component.enabled && this.isUICanvas(component) && component._isRootCanvas) {
  return component;
}

---

136-143: Add comments explaining matrix transformations.

The matrix operations for projection and view transformations would benefit from documentation explaining:

• The purpose of each transformation
• The coordinate space being used
• The significance of the magic numbers (2, 0)

packages/core/src/ui/UIRenderer.ts (2)

59-61: Consider omitting redundant type annotations.

The type annotations can be inferred from the initializers and can be omitted for better conciseness.

Apply these changes:

-protected _alpha: number = 1;
+protected _alpha = 1;

-protected _color: Color = new Color(1, 1, 1, 1);
+protected _color = new Color(1, 1, 1, 1);

---

229-241: Improve variable naming in _hitTest method.

Consider using more descriptive variable names for better readability.

Apply these changes:

 protected _hitTest(localPosition: Vector3): boolean {
   const { x, y } = localPosition;
   const uiTransform = <UITransform>this._transform;
-  const { x: width, y: height } = uiTransform.size;
-  const { x: pivotX, y: pivotY } = uiTransform.pivot;
+  const { x: elementWidth, y: elementHeight } = uiTransform.size;
+  const { x: pivotOffsetX, y: pivotOffsetY } = uiTransform.pivot;
   const { x: paddingLeft, y: paddingBottom, z: paddingRight, w: paddingTop } = this.raycastPadding;
   
   return (
-    x >= -width * pivotX + paddingLeft &&
-    x <= width * (1 - pivotX) - paddingRight &&
-    y >= -height * pivotY + paddingTop &&
-    y <= height * (1 - pivotY) - paddingBottom
+    x >= -elementWidth * pivotOffsetX + paddingLeft &&
+    x <= elementWidth * (1 - pivotOffsetX) - paddingRight &&
+    y >= -elementHeight * pivotOffsetY + paddingTop &&
+    y <= elementHeight * (1 - pivotOffsetY) - paddingBottom
   );
 }

packages/core/src/input/pointer/emitter/PointerUIEventEmitter.ts (3)

223-232: Improve readability and maintainability of path composition

Several improvements can be made to the _composedPath method:

1. Avoid assignment within expressions
2. Add early return for invalid input
3. Consider extracting the path composition logic to a separate method for better reusability

Apply this diff to improve the code:

 private _composedPath(element: Component, path: Entity[]): Entity[] {
   if (!element) {
     path.length = 0;
     return path;
   }
-  let entity = (path[0] = element._entity);
+  path[0] = element._entity;
+  let entity = element._entity;
   let i = 1;
   if (element._componentType === ComponentType.UICanvas && (<UICanvas>element)._isRootCanvas) {
     path.length = 1;
     return path;
   } else {
     const rootEntity = (<UICanvas | UIRenderer>element)._rootCanvas._entity;
     for (; i < PointerUIEventEmitter._MAX_PATH_DEPTH && !!entity && entity !== rootEntity; i++) {
       entity = path[i] = entity.parent;
     }
   }
   path.length = i;
   return path;
 }

🧰 Tools

🪛 Biome

[error] 223-223: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

---

170-171: Consider performance optimizations for path operations

The current implementation creates new paths and performs comparisons frequently, which could impact performance with high-frequency pointer events. Consider:

1. Caching paths when possible
2. Using a more efficient data structure for path comparison
3. Implementing path pooling to reduce garbage collection

---

13-21: Enhance documentation and type safety

The class could benefit from:

1. JSDoc documentation for the class explaining its purpose and usage
2. Type annotations for event handlers
3. Documentation of the relationship between different pointer states
4. Interface definitions for the event data structure

packages/core/src/input/pointer/PointerManager.ts (5)

4-12: Well-structured architectural improvements!

The introduction of ClearableObjectPool and event emitters shows good separation of concerns and memory management practices. The modular approach with separate emitter classes will make the code more maintainable and testable.

Also applies to: 41-41

---

99-110: Consider caching physics initialization state

The _engine._physicsInitialized check on each pointer creation could lead to inconsistent behavior if physics initialization state changes during runtime. Consider caching this state during PointerManager initialization.

 constructor(engine: Engine, target: EventTarget) {
   // ... existing code ...
+  this._physicsEnabled = engine._physicsInitialized;
 }

 // Later in the code
-_engine._physicsInitialized && pointer._addEmitters(PointerPhysicsEventEmitter, _eventPool);
+this._physicsEnabled && pointer._addEmitters(PointerPhysicsEventEmitter, _eventPool);

---

224-255: Consider using event type constants

The string literals for event types could be replaced with constants to prevent typos and improve maintainability. Consider creating an enum or constant object for event types.

const PointerEventTypes = {
  DOWN: 'pointerdown',
  UP: 'pointerup',
  MOVE: 'pointermove',
  LEAVE: 'pointerleave',
  CANCEL: 'pointercancel'
} as const;

---

282-288: Optimize cleanup operations

Consider combining the array clearing operations for better performance. You could create a helper method to clear multiple arrays at once.

private _clearArrays() {
  [this._nativeEvents, this._pointers, this._downList, 
   this._downMap, this._upList, this._upMap].forEach(arr => arr.length = 0);
}

---

41-41: Document object pool configuration

Consider adding JSDoc comments to explain the object pool's configuration and lifecycle, particularly its garbage collection strategy and pool size management.

packages/core/src/ui/UIImage.ts (2)

196-216: Optimize property access and null checks.

Multiple property accesses and null checks can be optimized by destructuring and combining conditions.

 protected override _render(context: RenderContext): void {
-  const { _sprite: sprite } = this;
-  const transform = this._transform as UITransform;
-  const { x: width, y: height } = transform.size;
-  if (!sprite?.texture || !width || !height) {
+  const { _sprite: sprite, _transform: transform, _color: color, _alpha: alpha } = this;
+  if (!sprite?.texture || !transform) {
     return;
   }
+  const { x: width, y: height } = (transform as UITransform).size;
+  if (!width || !height || color.a * alpha <= 0) {
+    return;
+  }

   let material = this.getMaterial();
   if (!material) {
     return;
   }
   // @todo: This question needs to be raised rather than hidden.
   if (material.destroyed) {
     material = this._engine._basicResources.uiDefaultMaterial;
   }

-  if (this._color.a * this._alpha <= 0) {
-    return;
-  }

---

362-371: Consider using TypeScript const enum for better performance.

The ImageUpdateFlags enum would benefit from being a const enum as it's used internally for bitwise operations.

-enum ImageUpdateFlags {
+const enum ImageUpdateFlags {
   UV = 0x20,

   /** LocalPosition | WorldPosition | UV */
   AllPositionAndUV = 0x23,
   /** LocalPosition | WorldPosition | UV | Color */
   AllPositionUVAndColor = 0x33,
   /** LocalPosition | WorldPosition | UV | Color | LocalBounds | WorldBounds */
   All = 0x3f
 }

packages/core/src/ui/UIText.ts (3)

235-241: Consider adding error handling in the constructor.

The constructor assumes engine._textDefaultFont and engine._basicResources.textDefaultMaterial will always exist. Consider adding null checks and throwing appropriate errors if these dependencies are missing.

 constructor(entity: Entity) {
   super(entity);

   const { engine } = this;
+  if (!engine._textDefaultFont) {
+    throw new Error("Default font not initialized in engine.");
+  }
   this.font = engine._textDefaultFont;
+  if (!engine._basicResources?.textDefaultMaterial) {
+    throw new Error("Default text material not initialized in engine resources.");
+  }
   this.setMaterial(engine._basicResources.textDefaultMaterial);
 }

---

257-257: Improve readability by avoiding assignment in expression.

The assignment within the logical AND expression makes the code harder to read.

- this._subFont && (this._subFont = null);
+ if (this._subFont) {
+   this._subFont = null;
+ }

🧰 Tools

🪛 Biome

[error] 257-257: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

---

324-363: Consider performance optimizations in render loop.

The render method performs several operations that could be optimized:

1. Multiple property accesses could be cached
2. Array iterations could be optimized
3. Object allocations in render loop should be minimized

Consider these optimizations:

1. Cache frequently accessed properties outside the loop
2. Use pre-allocated arrays for temporary calculations
3. Consider implementing object pooling for SubRenderElements
4. Profile the rendering performance with large text content to identify bottlenecks

Would you like me to provide a detailed implementation of these optimizations?

packages/core/src/Entity.ts (1)

108-113: Consider adding type safety for transform access.

While the refactoring from public property to private property with getter is good for encapsulation, consider adding runtime type checking to prevent potential issues with transform types.

 get transform(): Transform {
+  if (!this._transform) {
+    throw new Error('Transform component is missing');
+  }
   return this._transform;
 }

packages/core/src/ui/interactive/transition/Transition.ts (2)

6-6: Avoid using 'any' as default type for generics to maintain type safety

The generic type parameters T and K default to any, which reduces the benefits of TypeScript's type checking. By specifying more precise default types or removing the defaults, you can enhance type safety and catch potential errors at compile time.

---

114-119: Ensure _countDown does not become negative in _onUpdate

In the _onUpdate() method, _countDown is decremented by delta, but there's no check to prevent it from becoming negative. Negative _countDown values could cause unintended behavior in the transition logic.

Consider clamping _countDown to zero after decrementing to prevent negative values.

Suggested change:

if (this._countDown > 0) {
- this._countDown -= delta;
+ this._countDown = Math.max(0, this._countDown - delta);
  this._updateValue();
}

packages/core/src/ui/interactive/UIInteractive.ts (1)

115-115: Separate assignments for better readability

Assigning multiple variables in a single statement can reduce readability. Consider splitting the assignments to improve clarity.

Apply this diff:

-        this._isPointerInside = this._isPointerDown = false;
+        this._isPointerInside = false;
+        this._isPointerDown = false;

packages/core/src/ui/UICanvas.ts (5)

89-89: Use if statement instead of logical operators for clarity

Using logical operators like && for control flow can reduce code readability. Consider using an explicit if statement to make the code clearer.

Apply this diff to enhance readability:

- (referenceResolution.x !== val.x || referenceResolution.y !== val.y) && referenceResolution.copyFrom(val);
+ if (referenceResolution.x !== val.x || referenceResolution.y !== val.y) {
+   referenceResolution.copyFrom(val);
+ }

---

142-143: Use if statement instead of logical operators for side effects

Using logical operators like && to trigger side effects can make the code harder to read and understand. Replace it with an if statement for better clarity.

Apply this diff to improve code clarity:

- this._realRenderMode === CanvasRenderMode.ScreenSpaceOverlay &&
-   this.scene._componentsManager._overlayCanvasesSortingFlag;
+ if (this._realRenderMode === CanvasRenderMode.ScreenSpaceOverlay) {
+   this.scene._componentsManager._overlayCanvasesSortingFlag = true;
+ }

---

154-154: Use if statement instead of logical operators for function calls

Using logical operators like && to conditionally call functions can reduce code readability. Consider using an explicit if statement for better clarity.

Apply this diff to enhance readability:

- this._realRenderMode === CanvasRenderMode.ScreenSpaceCamera && this._adapterPoseInScreenSpace();
+ if (this._realRenderMode === CanvasRenderMode.ScreenSpaceCamera) {
+   this._adapterPoseInScreenSpace();
+ }

---

194-194: Avoid assignment within an expression for better readability

Assigning values within expressions can be confusing and reduce code clarity. Separate the assignment from the declaration to improve readability.

Apply this diff to enhance code clarity:

- const renderElement = (this._renderElement = engine._renderElementPool.get());
+ this._renderElement = engine._renderElementPool.get();
+ const renderElement = this._renderElement;

🧰 Tools

🪛 Biome

[error] 194-194: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

---

359-359: Avoid assignment within a logical expression

Assigning a value within a logical expression can reduce code readability. Use an explicit if statement instead.

Apply this diff to improve clarity:

- child.isActive && (depth = this._walk(child, elements, depth));
+ if (child.isActive) {
+   depth = this._walk(child, elements, depth);
+ }

🧰 Tools

🪛 Biome

[error] 359-359: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check the definition of GroupModifyFlags
# Expected: Find the type definition and its possible values

# Search for GroupModifyFlags definition
echo "Searching for GroupModifyFlags definition:"
ast-grep --pattern 'enum GroupModifyFlags {
  $$$
}'

# Backup search using ripgrep
echo -e "\nBackup search for GroupModifyFlags:"
rg -A 5 "GroupModifyFlags"

#!/bin/bash
# Search for other transition implementations to verify if this binary approach is consistent
ast-grep --pattern 'class $_Transition extends Transition<$_, $_> {
  $$$
  protected override _updateCurrentValue($_, $_, weight: number): void {
    $$$
  }
  $$$
}'

#!/bin/bash
# Let's try a different approach to find transition implementations
rg "_updateCurrentValue.*weight.*number" -A 5

# Also search for the base Transition class to understand the expected behavior
rg "class Transition[^a-zA-Z]" -A 10

  protected override _applyValue(value: Sprite): void {
+   if (value && !this.isValidSprite(value)) {
+     throw new Error('Invalid sprite value provided');
+   }
    this._target.sprite = value || this._normal;
  }
+ 
+ private isValidSprite(value: any): value is Sprite {
+   return value && typeof value === 'object';
+   // Add additional sprite validation logic
+ }

#!/bin/bash
# Description: Verify that all exported files exist and export the expected components

# Test: Check if all imported files exist
echo "Checking if all files exist..."
for file in "UIGroup" "enums/CanvasRenderMode" "enums/ResolutionAdaptationStrategy" \
           "Button" "UICanvas" "UIImage" "UIRenderer" "UIText" "UITransform" \
           "interactive/transition/ColorTransition" "interactive/transition/ScaleTransition" \
           "interactive/transition/SpriteTransition" "interactive/transition/Transition"; do
  if ! fd -p "packages/core/src/ui/$file.ts"; then
    echo "Missing file: $file"
  fi
done

# Test: Verify exports in each file
echo "Checking exports in each file..."
for file in $(fd -e ts . "packages/core/src/ui"); do
  echo "Checking $file..."
  ast-grep --pattern 'export class $name' "$file"
  ast-grep --pattern 'export enum $name' "$file"
  ast-grep --pattern 'export interface $name' "$file"
done

#!/bin/bash
# Check specific exports in each file

# Check UIGroup export
echo "Checking UIGroup..."
rg "export (class|enum|interface) UIGroup" packages/core/src/ui/UIGroup.ts -A 1

echo "Checking enums..."
rg "export enum (CanvasRenderMode|ResolutionAdaptationStrategy)" packages/core/src/ui/enums/ -A 1

echo "Checking UI components..."
for component in "Button" "UICanvas" "UIImage" "UIRenderer" "UIText" "UITransform"; do
  echo "Checking $component..."
  rg "export (class|interface) $component" "packages/core/src/ui/$component.ts" -A 1
done

echo "Checking transitions..."
for transition in "ColorTransition" "ScaleTransition" "SpriteTransition" "Transition"; do
  echo "Checking $transition..."
  rg "export (class|interface) $transition" "packages/core/src/ui/interactive/transition/$transition.ts" -A 1
done

 _raycast(ray: Ray, out: HitResult, distance: number = Number.MAX_SAFE_INTEGER): boolean {
   const plane = UIRenderer._tempPlane;
   const { transform } = this._entity;
   const { worldForward, worldMatrix, worldPosition } = transform;
   const normal = plane.normal.copyFrom(worldForward);
   plane.distance = -Vector3.dot(normal, worldPosition);
   
-  const curDistance = ray.intersectPlane(plane);
-  if (curDistance >= 0 && curDistance < distance) {
+  const intersectionDistance = ray.intersectPlane(plane);
+  if (intersectionDistance < 0 || intersectionDistance >= distance) {
+    return false;
+  }

-    const hitPointWorld = ray.getPoint(curDistance, UIRenderer._tempVec30);
+  const hitPointWorld = ray.getPoint(intersectionDistance, UIRenderer._tempVec30);
   const worldMatrixInv = UIRenderer._tempMat;
-  Matrix.invert(transform.worldMatrix, worldMatrixInv);
+  Matrix.invert(worldMatrix, worldMatrixInv);
   
   const localPosition = UIRenderer._tempVec31;
   Vector3.transformCoordinate(hitPointWorld, worldMatrixInv, localPosition);
   
-  if (this._hitTest(localPosition)) {
-    out.distance = curDistance;
-    out.entity = this._entity;
-    out.component = this;
-    out.normal.copyFrom(normal);
-    out.point.copyFrom(hitPointWorld);
-    return true;
-  }
+  if (!this._hitTest(localPosition)) {
+    return false;
   }
-  return false;
+  
+  out.distance = intersectionDistance;
+  out.entity = this._entity;
+  out.component = this;
+  out.normal.copyFrom(normal);
+  out.point.copyFrom(hitPointWorld);
+  return true;
 }

 case SpriteDrawMode.Tiled:
-  return SlicedSpriteAssembler.getUVByLocalPosition(this, size.x, size.y, pivot, position, out);
+  return TiledSpriteAssembler.getUVByLocalPosition(this, size.x, size.y, pivot, position, out);